Move `pathfinder` to cuda-python top level #723

rwgk · 2025-06-24T08:20:15Z

Description

Note: The name is changed from path_finder to pathfinder. The word "pathfinder" is very common, and it avoids underscore/dash confusion between directory names and package names.

Move cuda.bindings.path_finder → cuda.pathfinder, with backward compatibility: cuda.bindings.path_finder forwards to cuda.pathfinder.
- Make LoadedDL._handle private (underscore prefix). It is now a platform-agnostic unsigned integer.
Make cuda-bindings dependent on cuda-pathfinder, with a temporary backward compatibility layer. — cuda.bindings.pathfinder is slated to be removed after the next cuda-bindings release.
Remove 32-bit DLL names from SUPPORTED_WINDOWS_DLLS and add runtime guard in cuda.pathfinder.load_nvidia_dynamic_lib(): "requires 64-bit Python".
Add specific DynamicLibNotFoundError exception type to public API (this was RuntimeError before; now inherits from RuntimeError).
Run mypy-pathfinder from pre-commit (after systematic mypy cleanup of library code).
Adjust GitHub Actions jobs to the move.
Add nvidia_wheels_cu12 to [project.optional-dependencies] in cuda_pathfinder/pyproject.toml and ensure all libs are loaded successfully. Treat all NVIDIA libs as "supported".
Mark cuda.pathfinder as version 1.0.0

…HOUT trying to make anything work.

… trying to make anything work.

…make anything work.

…ns.abc.Sequence (Python 3.9+)

…init__.py (to make the dynamic version work).

copy-pr-bot · 2025-06-24T08:20:19Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

…vidia_dynamic_lib

shwina · 2025-06-25T10:52:46Z

cuda_pathfinder/cuda/pathfinder/__init__.py

+def load_nvidia_dynamic_lib(libname: str) -> LoadedDL:
+    """Load a NVIDIA dynamic library by name.
+
+    Args:
+        libname: The name of the library to load (e.g. "cudart", "nvvm", etc.)
+
+    Returns:
+        A LoadedDL object containing the library handle and path
+
+    Raises:
+        RuntimeError: If the library cannot be found or loaded
+    """
+    return _load_nvidia_dynamic_lib.load_lib(libname)


You could define this wrapper function in the _dynamic_libs module and just import it here. This way your __init__.py here wouldn't have a ton of function definitions.

This file is meant to be the public API in one view.

This way your __init__.py here wouldn't have a ton of function definitions.

I understood exactly that was your goal: a flat list of available APIs.

Note that I moved the docstring here.

This file is meant to be the public API in one view.

Indeed, everything we define (or import) here that is not prefixed with _ constitutes the public API of the module cuda.path_finder. My above suggestion is just that we import load_nvidia_dynamic_lib rather than define it here.

That would make the public API far less obvious. E.g. to see the docstring, they'd need to open a private file.

I don't think that's true. They could still do:

from cuda.path_finder import load_nvidia_dynamic_lib help(load_nvidia_dynamic_lib)

Same as they would do now?

That'll only work interactively.

What I have now can be inspected in obvious ways directly in the sources, e.g. when looking at the sources on github. I can send URLs pointing to specific APIs in this init.py file.

Each function here will just be:

def function(...) -> ...: """docstring"" return call_into_private_code(...)

That's exactly the public API, with a one-line call that's easy to ignore.

What's the point of hiding that away, especially hiding away the docstring and the type hints?

That'll only work interactively.

The docstrings and type hints would also get picked up:

Sphinx (which we use to generate API docs
-pydoc

the developers' IDEs/lsp

Those are primarily the ways consumers of a package interact with docstrings or type hints, rather than looking directly at the source.

What's the point of hiding that away, especially hiding away the docstring and the type hints?

It tightens the scope of __init__.py, whose job is:

to include any initialization code for the module

to import stuff from submodules that the module wants to expose

to define __all__ for the module if needed

As examples, we can look at the __init__.py from some other popular libraries:

requests

pandas

It tightens the scope of init.py, whose job is

The most important job: Show the public API

You didn't answer why you want to hide that away.

I'm advocating for keeping function and type definitions outside of __init__.py. I don't think that "hides" anything from the user as we still import them here.

The main reason I'm advocating for that is because __init__.py typically only defines functions and types needed for module initialization, and imports anything else. I think the examples I linked to above are a good demonstration of that.

Defining functions and types beyond that serves to clutter __init__.py.

I would argue it makes the code base less navigable than more for people looking at the source. Do I expect the function load_nvidia_dynamic_libs to be defined in a file called _dynamic_libs.py, or a file called __init__.py?

If this all seems nitpicky, I apologize. While I do have a strong opinion here, I don't mind at all if another reviewer (or you, as the author of this PR) made the final call about this.

cuda.core's __init__.py is a good example for what Ashwin suggested. As a developer who occasionally peeks into someone else's __init__.py, I certainly get confused why we are defining things in there directly, as opposed to properly organizing them in respective public/private modules, and just import them. What's imported are considered public APIs (including types). Does it make sense?

rwgk · 2025-06-25T21:49:30Z

I'm a bit lost, to be honest, and I’d appreciate some clarification so I can move this forward in the direction you want.

The goals that seemed most important to me originally (from a Slack post last Wednesday) were:

Dependency isolation – avoid importing unrelated logic (e.g. dynamic lib loading) when only working with headers
Public API should be obvious from the code – minimal need for external docs or tooling to understand what’s exposed

After Ashwin’s comment on Slack, I interpreted the request as: prioritize a flat public API, even if that means giving up on some of that isolation — especially when type hints are involved.

Then Leo wrote:

as opposed to properly organizing them in respective public/private modules

That seems more aligned with what I had before the last commit (e.g., a file like nvidia_dynamic_libs.py directly under pathfinder/). But that seemed in conflict with Ashwin’s preference for flatness.

I’m happy to implement what you both want here — I just need a bit more clarity.

If we go back to what I proposed last week:

from cuda.pathfinder import nvidia_dynamic_libs, nvidia_static_libs, nvidia_headers

nvidia_dynamic_libs.load("nvrtc")
nvidia_static_libs.find("cudart")
nvidia_headers.find_file("cuda.h")

The file structure was:

cuda/pathfinder/
├── __init__.py  # empty
├── nvidia_dynamic_libs.py
├── nvidia_static_libs.py
└── nvidia_headers.py

I thought that structure balanced modularity and clarity, and users could still import just what they needed. But it sounds like we might instead prefer:

from cuda.pathfinder import load_nvidia_dynamic_lib, find_nvidia_static_lib, find_nvidia_header_file

That keeps the API flat, but does give up on the isolation goal unless we resort to function-local imports or other workarounds.

Could you please help me understand: What would be the preferred file/module structure to go along with the flat API? Should I keep separate modules like nvidia_dynamic_libs.py and re-export in __init__.py, or move everything into a single file (what would be the name)?

shwina · 2025-06-25T22:57:32Z

Given there's some urgency for getting this PR in, @rwgk @leofang do you want to proceed with the PR as is and address the questions Ralf posed above as a follow-up?

leofang · 2025-06-26T00:07:51Z

At this point we need to follow up offline. I think there is a gap that we unfortunately did not really cover during the weekly meeting. I suggest we don't cancel the dev meeting tomorrow and use the time to finalize it.

This PR also needs at least @kkraus14's approval for the change of license.

kkraus14 · 2025-07-11T18:11:23Z

This looks good to me, sans the one comment above about the visibility of things in the cuda.bindings.path_finder and avoiding accidentally introducing public APIs that we don't wish to support.

rwgk · 2025-07-11T20:11:33Z

/ok to test

rwgk · 2025-07-11T20:13:58Z

@kkraus14 I merged in main (SPDX changes) and put #751 as-is on top here: commit c0509be

Everything else is as before. I think this is ready for merging (pending completion of tests).

@leofang Merging is blocked by your request for changes, could you please help?

rwgk · 2025-07-11T23:48:08Z

I just added one more commit to this PR, just one extra line: eedf658

After reading up on publishing to PyPI, I'm guessing this is all that's needed on this end to publish cuda-pathfinder wheels to PyPI?

rwgk · 2025-07-12T04:39:32Z

I was thinking of making a release right after this is merged. This whole file is already deprecated per the README. I guess we could just add more underscores, would that be acceptable? Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Keith Kraus ***@***.***> Sent: Friday, July 11, 2025 9:24:22 PM To: NVIDIA/cuda-python ***@***.***> Cc: Ralf Grosse Kunstleve ***@***.***>; Mention ***@***.***> Subject: Re: [NVIDIA/cuda-python] Move `pathfinder` to cuda-python top level (PR #723) @kkraus14 commented on this pull request.

________________________________ In cuda_bindings/cuda/bindings/path_finder.py<#723 (comment)>:

__all__ = [

"_load_nvidia_dynamic_library", "_SUPPORTED_LIBNAMES", ] I do not think it's fine. We're following strict semver here, which means if you can publicly access something via cuda.bindings.path_finder that doesn't begin with an underscore that it is part of our API and we will not break it in bumping minor versions. If this is only a temporary stopgap that we don't intend to survive until a release then that is fine. — Reply to this email directly, view it on GitHub<#723 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAFUZACZPGCREHK5PLRHQYT3ICEXNAVCNFSM6AAAAAB77RWJV2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMJSGYZTKMRRG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

…backward_compatibility.py, to fully sanitize path_finder.py

rwgk · 2025-07-12T17:17:18Z

/ok to test

cuda_pathfinder/mypy.ini

cuda_bindings/cuda/bindings/_path_finder/README.md

cuda_bindings/cuda/bindings/path_finder.py

cuda_pathfinder/cuda/pathfinder/README.md

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_nvidia_dynamic_lib.py

cuda_pathfinder/tests/conftest.py

toolshed/build_pathfinder_dlls.py

- Move mypy configuration from cuda_pathfinder/mypy.ini to [tool.mypy] section in pyproject.toml - Delete cuda_pathfinder/mypy.ini file - Update .pre-commit-config.yaml to point to pyproject.toml for mypy configuration - Maintain identical functionality with consolidated config Addresses review comment about preferring pyproject.toml over separate ini files.

…ersion release." Also fix pathfinder → path_finder oversights.

…hat this wasn't removed before).

rwgk · 2025-07-14T19:50:05Z

I'm not rerunning the CI:

pre-commit.ci exercises the pyproject.toml changes.
All other changes only affect comments and documentation.

leofang

LGTM, thanks for pushing this forward Ralf!

btw I skipped reviewing commit c0509be from #751, unfortunately it's too much for me to review at once. In the future I'd appreciate we follow what we did in the past and have a series of smaller PRs with a focused scope.

leofang · 2025-07-15T14:56:42Z

.github/workflows/test-wheel-linux.yml

@@ -286,6 +292,11 @@ jobs:
      - name: Set up compute-sanitizer
        run: setup-sanitizer

+      - name: Run cuda.pathfinder tests with see_what_works


Later the pathfinder will need to check the driver version, so the status quo is fine.

leofang · 2025-07-15T15:03:52Z

toolshed/build_pathfinder_dlls.py

+        if dll.startswith("nvrtc-builtins"):
+            return True


For NVRTC wheel to work on Windows, this DLL is needed, e.g.
https://github.com/NVIDIA/nvmath-python/blob/e082fedb46a57072e63d3b8f0aae3b451eeddcbb/nvmath/_utils.py#L139

Maybe I just forget what "suppressed DLLs" means...

leofang · 2025-07-15T15:15:57Z

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_nvidia_dynamic_lib.py

+    pointer_size_bits = struct.calcsize("P") * 8
+    if pointer_size_bits != 64:


FYI usually we can check this with sys.maxsize > 2**32:
https://docs.python.org/3/library/platform.html#platform.architecture

leofang · 2025-07-15T15:23:02Z

btw @rwgk could you create an issue to track the need to add docs for pathfinder? We need a Sphinx-rendered docs.

rwgk · 2025-07-15T17:41:49Z

/ok to test

rwgk · 2025-07-15T17:47:05Z

btw I skipped reviewing commit c0509be from #751, unfortunately it's too much for me to review at once. In the future I'd appreciate we follow what we did in the past and have a series of smaller PRs with a focused scope.

Just to explain, that's my preferred work style, too, but two things led to the current situation:

[cuda_pathfinder] Remove win32api dependency (use ctypes instead). #751 removes the pywin32 dependency (f328230), this seemed to be very valuable to have from the get-go.
GitHub (OSS world in general) doesn't have an easy to use "stacked PRs" feature. Juggling multiple PRs is time consuming.

github-actions · 2025-07-15T21:06:07Z

Doc Preview CI
Preview removed because the pull request was closed or merged.

rwgk added 15 commits June 22, 2025 11:09

Move files WITHOUT trying to make anything work.

fbdebfa

First pass changing imports from cuda.bindings → cuda.path_finder WIT…

731256d

…HOUT trying to make anything work.

Move README.md one level up.

6e0ec6c

Move find_sub_dirs.py → ../_utils/find_sub_dirs.py

07e1385

Move files from cuda_bindings/tests to cuda_path_finder/tests WITHOUT…

68aa8d5

… trying to make anything work.

First VERY ROUGH version of pyproject.toml

fafa998

Change imports in tests/ WITHOUT trying to make anything work.

4eff7d2

Clean up pyproject.toml (with the help of ChatGPT) WITHOUT trying to …

db0f540

…make anything work.

ruff automatic fix: Replace deprecated typing.Sequence with collectio…

9fea2e7

…ns.abc.Sequence (Python 3.9+)

Add dynamic = ["version"] in pyproject.toml and cuda/path_finder/__…

c494ef2

…init__.py (to make the dynamic version work).

Modernize [project] license line.

78b8c6d

Change dir/file names: path_finder → pathfinder

5690d96

Change imports: path_finder → pathfinder

81b9bd9

Fix up broken imports.

b9be5cb

add [project.optional-dependencies] test

79a8019

github-project-automation bot added this to CCCL Jun 24, 2025

github-project-automation bot moved this to Todo in CCCL Jun 24, 2025

leofang assigned rwgk Jun 24, 2025

leofang self-requested a review June 24, 2025 12:51

leofang added cuda.pathfinder Everything related to the cuda.pathfinder module enhancement Any code-related improvements P0 High priority - Must do! labels Jun 24, 2025

rwgk changed the title ~~Move path_finder to cuda-python top level~~ Move pathfinder to cuda-python top level Jun 24, 2025

rwgk added 2 commits June 24, 2025 17:18

Adopt cuda_core license.

975ba0b

cuda.pathfinder.nvidia_dynamic_libs.load_lib → cuda.pathfinder.load_n…

3678fb4

…vidia_dynamic_lib

shwina reviewed Jun 25, 2025

View reviewed changes

rwgk added 2 commits July 11, 2025 12:10

Merge branch 'main' into move_path_finder_to_top

933a889

Put the changes from NVIDIA#751 on top of this PR (723)

c0509be

Add cuda-pathfinder to options in .github/workflows/release.yml

eedf658

Move temporary backward compatibility code to _path_finder/temporary_…

58ca181

…backward_compatibility.py, to fully sanitize path_finder.py

kkraus14 previously approved these changes Jul 14, 2025

View reviewed changes

rwgk added 3 commits July 14, 2025 12:19

Change "slated" to "deprecated and slated ... in the next ... major v…

88c4ebc

…ersion release." Also fix pathfinder → path_finder oversights.

Remove mention of LoadedDL handle in README.md (it was an oversight t…

ee19b0c

…hat this wasn't removed before).

rwgk dismissed kkraus14’s stale review via ee19b0c July 14, 2025 19:33

leofang approved these changes Jul 15, 2025

View reviewed changes

github-project-automation bot moved this from In Progress to In Review in CCCL Jul 15, 2025

kkraus14 approved these changes Jul 15, 2025

View reviewed changes

rwgk merged commit ed12c83 into NVIDIA:main Jul 15, 2025
53 checks passed

github-project-automation bot moved this from In Review to Done in CCCL Jul 15, 2025

rwgk deleted the move_path_finder_to_top branch July 15, 2025 20:52

rwgk mentioned this pull request Jul 16, 2025

[FEA]: Improve toolshed/build_path_finder_dlls.py to minimize potential for accidents. #708

Closed

1 task

leofang mentioned this pull request Jul 18, 2025

Add Sphinx docs for cuda.pathfinder #760

Open

This was referenced Jul 18, 2025

Initial version of path_finder find_nvidia_headers.py (Minimal Viable Product) #661

Draft

Add cuda.pathfinder to Component, Area pull-downs in ISSUE_TEMPLATEs #777

Merged

		pointer_size_bits = struct.calcsize("P") * 8
		if pointer_size_bits != 64:

Move pathfinder to cuda-python top level #723

Move pathfinder to cuda-python top level #723

Uh oh!

Conversation

rwgk commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

copy-pr-bot bot commented Jun 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shwina Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shwina Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leofang Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwgk commented Jun 25, 2025

Uh oh!

shwina commented Jun 25, 2025

Uh oh!

leofang commented Jun 26, 2025

Uh oh!

kkraus14 commented Jul 11, 2025

Uh oh!

rwgk commented Jul 11, 2025

Uh oh!

rwgk commented Jul 11, 2025

Uh oh!

rwgk commented Jul 11, 2025

Uh oh!

rwgk commented Jul 12, 2025 via email

Uh oh!

rwgk commented Jul 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rwgk commented Jul 14, 2025

Uh oh!

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leofang commented Jul 15, 2025

Uh oh!

rwgk commented Jul 15, 2025

Uh oh!

rwgk commented Jul 15, 2025

Uh oh!

Move `pathfinder` to cuda-python top level #723

Move `pathfinder` to cuda-python top level #723

rwgk commented Jun 24, 2025 •

edited

Loading

shwina Jun 25, 2025 •

edited

Loading

shwina Jun 25, 2025 •

edited

Loading

leofang Jun 25, 2025 •

edited

Loading